Skip to content

implement persistence for child bodies during reset#1251

Open
Vareniel wants to merge 2 commits intomelonjs:masterfrom
Vareniel:laviruz-melonjs
Open

implement persistence for child bodies during reset#1251
Vareniel wants to merge 2 commits intomelonjs:masterfrom
Vareniel:laviruz-melonjs

Conversation

@Vareniel
Copy link
Contributor

@Vareniel Vareniel commented Mar 3, 2026

Description of change

Implement persistence for child bodies during reset.

The body of an object marked as isPersistent was being ignored by the world update during stage transitions. This occurred because this.bodies.clear() removed it without re-adding it during the reset() process. This fix ensures the body is retained.

Merge Checklist
  • [-] Build process passed (npm run build)
  • [-] Lint process passed (npm run lint)
  • [-] Tests passed (npm run test)

Copilot AI review requested due to automatic review settings March 3, 2026 07:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes physics-body persistence across world resets/stage transitions so bodies associated with persistent objects aren’t dropped from the world’s active body set.

Changes:

  • Capture a subset of bodies before this.bodies.clear() during World.reset().
  • Re-add captured bodies back into this.bodies after the reset.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +128 to +131
// save persistent child bodies
const persistentBodies = [];
this.bodies.forEach((value) => {
if (value.ancestor && value.ancestor.isPersistent) {
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The persistence filter only keeps bodies whose owning renderable is marked isPersistent. Bodies belonging to non-persistent renderables inside a persistent container will still remain in the scene after super.reset(), but will be dropped from this.bodies by the subsequent clear() and never re-added, causing those objects to stop being stepped/collided. Consider retaining any body whose body.ancestor is still attached to this world after super.reset() (e.g., via the renderable’s container chain/root ancestor), or rebuilding this.bodies from the post-reset scene graph instead of checking only isPersistent on the body ancestor.

Suggested change
// save persistent child bodies
const persistentBodies = [];
this.bodies.forEach((value) => {
if (value.ancestor && value.ancestor.isPersistent) {
// save bodies whose owning renderable is still attached to this world
const persistentBodies = [];
this.bodies.forEach((value) => {
const owner = value.ancestor;
if (!owner) {
return;
}
// Walk up the container chain to check if the owner is still under this world
let current = owner;
while (current && current !== this && current.ancestor) {
current = current.ancestor;
}
if (current === this) {

Copilot uses AI. Check for mistakes.
// insert persistent child bodies into the new state
if (persistentBodies.length > 0) {
persistentBodies.forEach((body) => {
return this.bodies.add(body);
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-adding bodies directly via this.bodies.add(...) bypasses the addBody method’s physic === "builtin" guard. Using addBody here would keep reset behavior consistent when physics is disabled and avoids populating this.bodies in modes where it isn’t used.

Suggested change
return this.bodies.add(body);
this.addBody(body);

Copilot uses AI. Check for mistakes.
Comment on lines +128 to +145
// save persistent child bodies
const persistentBodies = [];
this.bodies.forEach((value) => {
if (value.ancestor && value.ancestor.isPersistent) {
persistentBodies.push(value);
}
});

// empty the list of active physic bodies
// Note: this should be empty already when calling the parent method
this.bodies.clear();

// insert persistent child bodies into the new state
if (persistentBodies.length > 0) {
persistentBodies.forEach((body) => {
return this.bodies.add(body);
});
}
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces/changes behavior around which bodies survive World.reset(), but there doesn’t appear to be coverage for world reset + persistence in the current test suite. Adding a unit test that exercises a persistent container (with non-persistent children that have bodies) and/or a persistent renderable with a body would help prevent regressions.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants